Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Issue1844 move interpolate to math #1845

Merged
merged 13 commits into from
Mar 25, 2024
Merged

Issue1844 move interpolate to math #1845

merged 13 commits into from
Mar 25, 2024

Conversation

hcasperfu
Copy link
Contributor

This closes #1844.

@hcasperfu hcasperfu requested a review from mwetter February 29, 2024 16:58
@hcasperfu hcasperfu self-assigned this Feb 29, 2024
mwetter
mwetter requested changes Mar 5, 2024
View reviewed changes
Copy link
Contributor

@mwetter mwetter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hcasperfu : Can you please also make an entry in the conversion script. Afterwards it is ready for review. Note that a developer of each library needs to approve within a week (or raise concerns) as this is a non-backward compatible change.

@hcasperfu
Copy link
Contributor Author

@mwetter -
The two moved classes did not exist yet at v3.0.0, and therefore the conversion script shouldn't apply here.

@mwetter
Copy link
Contributor

mwetter commented Mar 6, 2024

@mwetter - The two moved classes did not exist yet at v3.0.0, and therefore the conversion script shouldn't apply here.

Thanks for checking. (In Buildings we likely need to add the rename to the conversion script.)

@hcasperfu
Copy link
Contributor Author

Can each library please review (or tag someone appropriate) and provide feedback by Friday March 22nd?
AixLib @FWuellhorst
BuildingSystems @nytschgeusen
IDEAS @jelgerjansen

@FWuellhorst
Copy link
Contributor

We don't use the function in AixLib, so it is fine for us.

@jelgerjansen
Copy link
Contributor

I checked in the IDEAS library and double checked with Ghent University, but we also don't use the function in one of our library-specific models.

@nytschgeusen
Copy link
Contributor

Also BuildingSystems doesn't use the interpolate function in its specific models ...

@hcasperfu
Copy link
Contributor Author

Thank you for all your feedback!

@mwetter -
There is no objection from the libraries. This PR is ready to merge.

@hcasperfu hcasperfu mentioned this pull request Mar 13, 2024
Merged
@mwetter mwetter merged commit 48254b3 into master Mar 25, 2024
3 checks passed
@mwetter mwetter deleted the issue1844_moveInterpolateToMath branch March 25, 2024 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mwetter mwetter mwetter approved these changes

Assignees

@hcasperfu hcasperfu

Labels
non-backward compatible
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Move Airflow.Multizone.BaseClasses.interpolate to Utilities.Math.Functions
5 participants
@hcasperfu @mwetter @FWuellhorst @jelgerjansen @nytschgeusen